Skip to content

REST: Add pagination support for list_views#3349

Open
ebyhr wants to merge 2 commits into
apache:mainfrom
ebyhr:ebi/rest-page-views
Open

REST: Add pagination support for list_views#3349
ebyhr wants to merge 2 commits into
apache:mainfrom
ebyhr:ebi/rest-page-views

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented May 9, 2026

Rationale for this change

Follows REST catalog spec:

Are these changes tested?

Yes, includes unit tests for paginated cases.

Are there any user-facing changes?

  • Before: returned incomplete view list when server paginated (only first page)
  • After: returns complete view list (fetches all pages)

Copy link
Copy Markdown
Member

@geruh geruh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising @ebyhr! I did a quick pass. Overall looks good, left a few comments.

Comment thread pyiceberg/catalog/rest/__init__.py Outdated
Comment thread pyiceberg/catalog/rest/__init__.py Outdated
assert RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).list_views(namespace) == [("examples", "fooshare")]


def test_list_views_paginated_200(rest_mock: Mocker) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests for an ommitted field and null for the next_page_token, and ensure the behavior is the same?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test (test_list_views_paginated_200) verifies the omitted next-page-token field. Added another test with "next-page-token": None.

Let me know if you want me to add more tests.

@ebyhr ebyhr force-pushed the ebi/rest-page-views branch from 9b3e652 to 553a73a Compare May 11, 2026 06:11
@ebyhr ebyhr force-pushed the ebi/rest-page-views branch from 553a73a to efcbf5b Compare May 11, 2026 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants